Remove Fabric and TurboModule dead config - #58434
Open
christophpurrer wants to merge 3 commits into
Open
Conversation
…eact#58415) Summary: Bridgeless is the only supported mode in the New Architecture, so the `bridgelessEnabled` flag on `DefaultNewArchitectureEntryPoint` was dead configuration: every `load(...)` path passed `true`, and `isConfigurationValid` raised an error when it was `false`. The entry point advertised a toggle that could only ever hold the one value that is already mandatory. This removes that surface from `ReactAndroid`: - Removed the public `bridgelessEnabled` getter and its backing field. - Removed the deprecated three-argument `load(turboModulesEnabled, fabricEnabled, bridgelessEnabled)` overload. Its implementation body moved into the two-argument overload, since dropping the parameter alone would have collided with the existing `load(Boolean, Boolean)` signature. - Removed the `bridgelessEnabled` parameter from `isConfigurationValid`, reducing the guard to `!turboModulesEnabled || !fabricEnabled` and shortening the resulting error message. - `loadWithFeatureFlags` no longer reads `enableBridgelessArchitecture()`. Behavior note: `loadWithFeatureFlags` previously raised an error when a feature flags provider returned `enableBridgelessArchitecture() == false`. That check is gone. It was unreachable in practice because bridgeless is not optional, but it is a removed validation rather than a pure no-op cleanup. Bridgeless remains unconditionally enabled. Callers using the no-argument `load()` are unaffected. The regenerated `ReactAndroid.api` drops exactly `getBridgelessEnabled ()Z`, `load (ZZZ)V`, and `load$default (ZZZILjava/lang/Object;)V`. The equivalent iOS cleanup is intentionally left to a follow-up change. Changelog: [Android][Breaking] - Remove `DefaultNewArchitectureEntryPoint.bridgelessEnabled` and the deprecated three-argument `load(turboModulesEnabled, fabricEnabled, bridgelessEnabled)` overload; bridgeless is always enabled in the New Architecture Reviewed By: rubennorte Differential Revision: D116317210
…act#58416) Summary: Follow-up to the Android change in the previous diff, applying the same cleanup to iOS. Bridgeless is the only supported mode, so the `bridgelessEnabled` surface on `RCTRootViewFactoryConfiguration` was already deprecated and hardcoded: the property was assigned `YES` in every initializer, the two deprecated initializers ignored the argument entirely, and `RCTDefaultReactNativeFactoryDelegate` returned `YES` unconditionally. Nothing read the value. Changes: - Removed the `bridgelessEnabled` property from `RCTRootViewFactoryConfiguration`. - Removed the two deprecated initializers `initWithBundleURLBlock:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` and `initWithBundleURL:newArchEnabled:turboModuleEnabled:bridgelessEnabled:`. Both were marked `__deprecated` and discarded all arguments except the bundle URL, delegating to the designated initializer. - Removed the `bridgelessEnabled` method from `RCTDefaultReactNativeFactoryDelegate`. It was not declared in any header or protocol. - Updated the one caller in `RCTReactNativeFactory` to use `initWithBundleURLBlock:newArchEnabled:`. Behavior is unchanged: bridgeless remains unconditionally enabled. Callers of the designated `initWithBundleURLBlock:newArchEnabled:` and `initWithBundleURL:newArchEnabled:` initializers are unaffected. Changelog: [iOS][Breaking] - Remove the deprecated `bridgelessEnabled` property and the deprecated `initWithBundleURLBlock:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` / `initWithBundleURL:newArchEnabled:turboModuleEnabled:bridgelessEnabled:` initializers from `RCTRootViewFactoryConfiguration`; bridgeless is always enabled Reviewed By: rubennorte Differential Revision: D116318829
Summary: Follow-up to D116318829, addressing rubennorte's review comment. Fabric and TurboModules shipped before bridgeless and are always on, so the toggles for them were hardcoded and read nowhere. Android, `DefaultNewArchitectureEntryPoint` — now only selects the release channel and loads the SO: - removed `fabricEnabled`, `turboModulesEnabled`, `concurrentReactEnabled` - removed the deprecated `load(turboModulesEnabled)` and `load(turboModulesEnabled, fabricEnabled)` overloads - removed `isConfigurationValid`, and with it `DefaultNewArchitectureEntryPointTest` (every test targeted it) - updated the 8 in-repo call sites that passed `fabricEnabled` into the deprecated 3-arg `DefaultReactActivityDelegate` constructor, which discarded it iOS: - removed `fabricEnabled` / `turboModuleEnabled` from `RCTRootViewFactoryConfiguration` - removed the corresponding `RCTDefaultReactNativeFactoryDelegate` stubs and the `RCTAppDelegate.h` doc references `ReactAndroid.api` and the `ReactApple*Cxx.api` snapshots are regenerated. One call site is not updated here: `users/zh/zhaogang/benchmarks/SimpleRN/android/app/src/main/java/com/simplern/MainActivity.kt` still imports `DefaultNewArchitectureEntryPoint.fabricEnabled`. It is a personal benchmark app under `users/` that is not materialized in this working copy, so it could not be edited. Changelog: [General][Breaking] - Remove the `fabricEnabled` / `turboModulesEnabled` / `concurrentReactEnabled` accessors and remaining deprecated `load` overloads from `DefaultNewArchitectureEntryPoint`, and the `fabricEnabled` / `turboModuleEnabled` properties from `RCTRootViewFactoryConfiguration`; Fabric and TurboModules are always enabled Differential Revision: D119380472
meta-cla
Bot
added
the
CLA Signed
|
@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119380472. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Follow-up to D116318829, addressing rubennorte's review comment. Fabric and TurboModules shipped before bridgeless and are always on, so the toggles for them were hardcoded and read nowhere.
Android,
DefaultNewArchitectureEntryPoint— now only selects the release channel and loads the SO:fabricEnabled,turboModulesEnabled,concurrentReactEnabledload(turboModulesEnabled)andload(turboModulesEnabled, fabricEnabled)overloadsisConfigurationValid, and with itDefaultNewArchitectureEntryPointTest(every test targeted it)fabricEnabledinto the deprecated 3-argDefaultReactActivityDelegateconstructor, which discarded itiOS:
fabricEnabled/turboModuleEnabledfromRCTRootViewFactoryConfigurationRCTDefaultReactNativeFactoryDelegatestubs and theRCTAppDelegate.hdoc referencesReactAndroid.apiand theReactApple*Cxx.apisnapshots are regenerated.One call site is not updated here:
users/zh/zhaogang/benchmarks/SimpleRN/android/app/src/main/java/com/simplern/MainActivity.ktstill importsDefaultNewArchitectureEntryPoint.fabricEnabled. It is a personal benchmark app underusers/that is not materialized in this working copy, so it could not be edited.Changelog:
[General][Breaking] - Remove the
fabricEnabled/turboModulesEnabled/concurrentReactEnabledaccessors and remaining deprecatedloadoverloads fromDefaultNewArchitectureEntryPoint, and thefabricEnabled/turboModuleEnabledproperties fromRCTRootViewFactoryConfiguration; Fabric and TurboModules are always enabledDifferential Revision: D119380472